refactor(ads-client): move a constructor onto its type and retire the common drawer - #7613

At first being a method on the impl felt a bit odd because it was static and felt a little bit javaish, but since it is returning an optional instance out of it it does make more sense. It's kinda like a try_from or try_into, but not quite either. Is there any sort of general interface that matches this that could simplify the concept? Something that takes a mutable ref and returns a new instance?

This makes sense to me. The 'ideal' place for such a mutable pop method would be on Url itself, but obviously that's not so possible here as we don't own Url. I can't think off-hand of an equivalent std constructor though where the passed field is mutable. We could newtype or create a trait to be able to put the function on Url but those seem like overkill to me- I'm fine with it either as a function or on RequestHash.

Sorry for the late response.

That specific change came from the free-function rule. When a free function takes or returns a local type, the rule expects that function to live on the type, usually in an impl block.

I agree this looks a lot like TryFrom, but it's less constrained. With TryFrom we'd have to switch the return type from Option to Result, which is annoying. The rule doesn't care which one we pick. We can use TryFrom, a custom trait or an associated function. The only thing it flags is a free function that is really a constructor.

So the rule doesn't conflict with TryFrom or custom traits, and it doesn't stop us from using them. Like the orphan rule, it only looks at local types. If none of the types are local, it won't fire, so fn add(a: i32, b: i32) -> i32 is still fine.